Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Move all E2E tests from tests/ to e2e/, renaming files to snake_case .spec.ts - Update playwright.config.ts: testDir, testMatch (spec only), remove dead commented blocks - Rename package.json script test:integration → test:e2e - Update AGENTS.md, .claude/rules/testing.md, docs, CI workflow accordingly Closes #3288 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughE2E test configuration, scripts, and docs were updated to use an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
playwright.config.ts (1)
16-16: Matcher is broader than the documented.spec.tsconvention.Line 16 currently matches both
.spec.tsand.spec.js. If the project standard is strictly.spec.ts, tighten this regex to enforce it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playwright.config.ts` at line 16, The testMatch pattern in playwright.config.ts (symbol: testMatch) is too broad—/((.+\.)?spec\.[jt]s)/—and currently allows .spec.js files; replace it with a stricter regex that only matches TypeScript spec files (e.g., ensure the pattern ends with .spec.ts) so only .spec.ts tests are discovered and run..github/workflows/ci.yml (1)
65-67: Rename the commented step label to match the command.Line 67 uses
pnpm test:e2e, but Lines 65-66 still say “Integration test.” Renaming that label to “E2E test” would keep terminology consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 65 - 67, Rename the commented GitHub Actions step label from "Integration test" to "E2E test" so it matches the command being run; update the commented block that currently shows the step name and command (the lines containing the label "Integration test" and the command "pnpm test:e2e") to use "E2E test" as the step name.AGENTS.md (1)
74-74: Consider adding the E2E filename convention here too.Line 74 is a key testing rule entrypoint; adding “E2E files use
.spec.ts” would keep this file aligned with the testing rule doc.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` at line 74, Update the "Tests" bullet to explicitly state the E2E filename convention by adding a short clause like "E2E files use `.spec.ts`" so the testing rules include naming guidance; modify the existing Tests line (the bullet that begins "**Tests**: Write tests before implementation (TDD). ...") to append this convention after the Nock/DB mocking guidance, ensuring the sentence reads clearly and keeps the existing guidance about using `@quramy/prisma-fabbrica` only in `prisma/seed.ts`, mocking the DB with `vi.mock('$lib/server/database', ...)`, and using Nock for HTTP mocking.docs/guides/architecture.md (1)
176-181: Clarify heading scope for test asset locations.Line 176 says common test assets remain in
src/test/, while Line 180 lists root-levele2e/. Consider rewording the heading so it doesn’t implye2e/is insidesrc/test/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guides/architecture.md` around lines 176 - 181, 見出し「共通テスト資産は src/test/ に残す」が `E2E テスト（e2e/）` を列挙しているため e2e/ が src/test/ 配下と誤解される可能性があります。見出しを「共通テスト資産の配置」や「共通テスト資産は src/test/ に配置（E2E はルートの e2e/）」のように書き換え、行内で `src/test/` と `e2e/` のスコープを明示してください（参照: 見出しテキスト "共通テスト資産は src/test/ に残す" と箇条書き項目 "E2E テスト（`e2e/`）" を修正）。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/guides/claude-code.md`:
- Around line 45-49: Update the testing scope table entry for `testing.md` to
include the missing `**/*.spec.ts` pattern alongside `*.test.ts` and `e2e/**`;
locate the `testing.md` row in the table (the line showing `*.test.ts`,
`e2e/**`) and append or replace it so it reads something like `*.test.ts,
**/*.spec.ts, e2e/**` to accurately reflect the testing rule.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 65-67: Rename the commented GitHub Actions step label from
"Integration test" to "E2E test" so it matches the command being run; update the
commented block that currently shows the step name and command (the lines
containing the label "Integration test" and the command "pnpm test:e2e") to use
"E2E test" as the step name.
In `@AGENTS.md`:
- Line 74: Update the "Tests" bullet to explicitly state the E2E filename
convention by adding a short clause like "E2E files use `.spec.ts`" so the
testing rules include naming guidance; modify the existing Tests line (the
bullet that begins "**Tests**: Write tests before implementation (TDD). ...") to
append this convention after the Nock/DB mocking guidance, ensuring the sentence
reads clearly and keeps the existing guidance about using
`@quramy/prisma-fabbrica` only in `prisma/seed.ts`, mocking the DB with
`vi.mock('$lib/server/database', ...)`, and using Nock for HTTP mocking.
In `@docs/guides/architecture.md`:
- Around line 176-181: 見出し「共通テスト資産は src/test/ に残す」が `E2E テスト（e2e/）` を列挙しているため
e2e/ が src/test/ 配下と誤解される可能性があります。見出しを「共通テスト資産の配置」や「共通テスト資産は src/test/ に配置（E2E
はルートの e2e/）」のように書き換え、行内で `src/test/` と `e2e/` のスコープを明示してください（参照: 見出しテキスト
"共通テスト資産は src/test/ に残す" と箇条書き項目 "E2E テスト（`e2e/`）" を修正）。
In `@playwright.config.ts`:
- Line 16: The testMatch pattern in playwright.config.ts (symbol: testMatch) is
too broad—/((.+\.)?spec\.[jt]s)/—and currently allows .spec.js files; replace it
with a stricter regex that only matches TypeScript spec files (e.g., ensure the
pattern ends with .spec.ts) so only .spec.ts tests are discovered and run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 064e3aed-0344-4d3c-b38a-5247a9ef70d5
📒 Files selected for processing (19)
.claude/rules/testing.md.claude/skills/session-close/instructions.md.github/workflows/ci.ymlAGENTS.mddocs/guides/architecture.mddocs/guides/claude-code.mde2e/about_page.spec.tse2e/custom_colors.spec.tse2e/dark_mode.spec.tse2e/helpers/auth.tse2e/navbar.spec.tse2e/robots.spec.tse2e/signin.spec.tse2e/sitemap.spec.tse2e/toppage.spec.tse2e/workbook_order.spec.tse2e/workbooks_list.spec.tspackage.jsonplaywright.config.ts
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
close #3288
Summary by CodeRabbit
Chores
Documentation